scripts/generate_simple_test_proof.sh-4-8 (1)

4-8: ⚠️ Potential issue | 🟠 Major

EXIT trap deletes proof files before the user can save them as fixtures.

The trap cleanup EXIT removes test_proof_simple.json and test_public_simple.json when the script exits, but lines 60–63 instruct the user to manually save those same files afterward. By the time the user sees the instructions, the files are already deleted.

Either exclude the proof/public files from cleanup, or integrate the fixture-saving step into the script itself before cleanup runs.

Proposed fix: exclude output files from cleanup

 cleanup() {
-  rm -f test_input_simple.json test_witness_simple.wtns test_proof_simple.json test_public_simple.json
+  rm -f test_input_simple.json test_witness_simple.wtns
 }

src/libs/l2ps/zk/snarkjs.d.ts-6-78 (1)

6-78: ⚠️ Potential issue | 🟠 Major

Remove this custom type declaration and use @types/snarkjs instead.

The @types/snarkjs package exists on DefinitelyTyped and is the community-maintained standard for snarkjs TypeScript types. Install it as a dev dependency (npm i -D @types/snarkjs``) and remove this custom .d.ts file to reduce maintenance burden and ensure compatibility with upstream updates.

src/libs/l2ps/zk/scripts/setup_all_batches.sh-28-41 (1)

28-41: ⚠️ Potential issue | 🟠 Major

Missing integrity check for downloaded PTAU files.

The PTAU files are critical cryptographic material for the trusted setup. Downloading over HTTPS without verifying a known checksum (SHA-256) means a compromised CDN or MITM proxy could serve malicious files, undermining the entire ZK proof system. Consider adding checksum verification after download.

🛡️ Proposed fix

 download_ptau() {
     local size=$1
     local file="powersOfTau28_hez_final_${size}.ptau"
     local url="https://storage.googleapis.com/zkevm/ptau/$file"
     
     if [[ ! -f "$PTAU_DIR/$file" ]] || [[ $(stat -c%s "$PTAU_DIR/$file") -lt 1000000 ]]; then
         echo -e "${YELLOW}Downloading pot${size}...${NC}"
         rm -f "$PTAU_DIR/$file"
         curl -L -o "$PTAU_DIR/$file" "$url"
+        # Verify integrity (add known checksums for each PTAU size)
+        echo "Verifying checksum for pot${size}..."
+        local expected_checksum
+        case $size in
+            16) expected_checksum="<sha256_for_pot16>" ;;
+            17) expected_checksum="<sha256_for_pot17>" ;;
+            *) echo -e "${YELLOW}⚠ No checksum available for pot${size}${NC}"; return 0 ;;
+        esac
+        local actual_checksum
+        actual_checksum=$(sha256sum "$PTAU_DIR/$file" | cut -d' ' -f1)
+        if [[ "$actual_checksum" != "$expected_checksum" ]]; then
+            echo -e "\033[0;31m❌ Checksum mismatch for pot${size}! Deleting file.${NC}"
+            rm -f "$PTAU_DIR/$file"
+            return 1
+        fi
     else
         echo "pot${size} already exists"
     fi
     return 0
 }

scripts/generate_test_proof.sh-4-8 (1)

4-8: ⚠️ Potential issue | 🟠 Major

Cleanup trap deletes proof files before the user can save fixtures.

The trap cleanup EXIT on line 8 removes test_proof.json and test_public.json as soon as the script exits — immediately after printing the "save the fixture" instructions on lines 99–100. The user never has a chance to run those commands.

Either copy the files into the fixtures directory before the trap fires, or exclude the proof/public files from cleanup when generation succeeds.

Proposed fix: auto-save fixtures on success

 echo "✅ All done! Now run these commands to save the fixture:"
 echo ""
-echo "   mkdir -p src/tests/fixtures"
-echo "   cat test_proof.json test_public.json | jq -s '{proof: .[0], publicSignals: .[1]}' > src/tests/fixtures/valid_proof_fixture.json"
+mkdir -p src/tests/fixtures
+cat test_proof.json test_public.json | jq -s '{proof: .[0], publicSignals: .[1]}' > src/tests/fixtures/valid_proof_fixture.json
+echo "   Saved to src/tests/fixtures/valid_proof_fixture.json"
 echo ""
 echo "Note: Temporary files will be cleaned up automatically on exit."
 echo ""

Also applies to: 97-102

src/libs/l2ps/zk/circuits/l2ps_batch_5.circom-26-27 (1)

26-27: ⚠️ Potential issue | 🟠 Major

check signal is a dead constraint — it does not enforce non-negative balances.

sender_after * sender_after in a prime field is always satisfiable for any field element; squaring doesn't prove non-negativity. The check signal is computed but never compared against a meaningful bound.

If the intent is to ensure sender_after >= 0 (i.e., prevent underflow), you need a range check via Num2Bits(n) to constrain sender_after to fit within n bits. Without this, a malicious prover could wrap around the field to fabricate balances.

If the batch aggregator is always trusted and this circuit only proves correct computation (not input validity), consider removing this dead signal to reduce confusion.

Option A: Remove the dead constraint

     receiver_after === receiver_before + amount;
     
-    signal check;
-    check <== sender_after * sender_after;
-    
     component preHasher = Poseidon(2);

Option B: Add a proper range check

+include "bitify.circom";
+
 template BalanceTransfer() {
     ...
     receiver_after === receiver_before + amount;
     
-    signal check;
-    check <== sender_after * sender_after;
+    // Range check: sender_after must fit in 64 bits (non-negative)
+    component senderCheck = Num2Bits(64);
+    senderCheck.in <== sender_after;
+
+    // Range check: amount must fit in 64 bits (non-negative)
+    component amountCheck = Num2Bits(64);
+    amountCheck.in <== amount;
     
     component preHasher = Poseidon(2);

src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts-1150-1156 (1)

1150-1156: ⚠️ Potential issue | 🟠 Major

Missing rollback cases for ZK operations.

When isRollback is true (line 1060–1062), the operation is flipped ("add" → "remove"). This means a rollback of "zk_commitmentadd" would try to match "zk_commitmentremove", and a rollback of "zk_attestationadd" would match "zk_attestationremove". Neither case exists in the switch, so rollbacks silently fall through to the default branch returning "Unsupported identity operation".

This means ZK commitments and attestations cannot be rolled back, which could leave orphaned state if a block is reverted.

Also applies to: 1164-1170

src/libs/l2ps/L2PSTransactionExecutor.ts-65-80 (1)

65-80: ⚠️ Potential issue | 🟠 Major

Failed initialization is never retried.

If the IIFE on line 72 rejects (e.g., Datasource connection fails), this.initPromise remains set to the rejected promise. All subsequent init() calls will await the same rejected promise and fail permanently. Consider resetting initPromise on failure:

Proposed fix

         this.initPromise = (async () => {
-            const dsInstance = await Datasource.getInstance()
-            const ds = dsInstance.getDataSource()
-            this.l1Repo = ds.getRepository(GCRMain)
-            log.info("[L2PS Executor] Repository initialized (unified state mode)")
+            try {
+                const dsInstance = await Datasource.getInstance()
+                const ds = dsInstance.getDataSource()
+                this.l1Repo = ds.getRepository(GCRMain)
+                log.info("[L2PS Executor] Repository initialized (unified state mode)")
+            } catch (error) {
+                this.initPromise = null
+                throw error
+            }
         })()

src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts-976-998 (1)

976-998: ⚠️ Potential issue | 🟠 Major

TOCTOU: account read outside transaction isolation.

ensureGCRForUser (line 980) reads the account using its own repository/connection, not the queryRunner.manager. The subsequent queryRunner.manager.save(account) (line 998) writes within the transaction, but the read was not part of that transaction. A concurrent operation modifying the same account between the read and write could cause lost updates (e.g., points overwritten).

Use the transactional manager for both read and write:

Proposed fix

-                const account = await ensureGCRForUser(editOperation.account)
+                // Read within transaction to prevent TOCTOU
+                let account = await queryRunner.manager.findOne(GCRMain, {
+                    where: { pubkey: editOperation.account }
+                })
+                if (!account) {
+                    account = await HandleGCR.createAccount(editOperation.account)
+                }

Note: This requires importing HandleGCR (or adapting ensureGCRForUser to accept an EntityManager).

src/index.ts-888-894 (1)

888-894: ⚠️ Potential issue | 🟠 Major

L2PS stop() calls are not awaited — async cleanup may be incomplete.

Per the code snippets, both L2PSHashService.stop(timeoutMs) and L2PSBatchAggregator.stop(timeoutMs) are async. Without await, process.exit(0) on Line 896 terminates before they finish. This issue is moot once the duplicate handlers above are resolved and the stops move into async gracefulShutdown.

src/libs/consensus/v2/PoRBFT.ts-245-246 (1)

245-246: ⚠️ Potential issue | 🟠 Major

Wrap L2PS rollback in try-catch to prevent cascading crash during error recovery.

If rollbackProofsForBlock throws here, it propagates to the outer catch which calls process.exit(1). A failure during rollback should be logged as a critical error but shouldn't prevent the remaining cleanup (the finally block still runs, but the return on line 248 is skipped).

Proposed fix

             // Also rollback any L2PS proofs that were applied
-            await L2PSConsensus.rollbackProofsForBlock(blockRef)
+            try {
+                await L2PSConsensus.rollbackProofsForBlock(blockRef)
+            } catch (rollbackError) {
+                log.error(`[consensusRoutine] Failed to rollback L2PS proofs: ${rollbackError}`)
+            }

src/features/zk/scripts/ceremony.ts-147-159 (1)

147-159: ⚠️ Potential issue | 🟠 Major

Corrupted state file silently treated as uninitialized — risk of ceremony re-initialization.

If ceremony_state.json exists but is corrupted (e.g., partial write), loadCeremonyState returns null. In initCeremony, this means the check on line 221 passes, and a new ceremony is initialized, overwriting the existing ceremony directory and potentially losing contributions.

Consider distinguishing "file doesn't exist" from "file exists but is corrupt":

Proposed fix

 function loadCeremonyState(): CeremonyState | null {
     if (!existsSync(STATE_FILE)) {
         return null
     }
 
     try {
         const content = readFileSync(STATE_FILE, "utf-8")
         return JSON.parse(content)
     } catch (err) {
-        warn(`Failed to parse ceremony state: ${err}`)
-        return null
+        error(`Ceremony state file exists but is corrupted: ${err}\nManually inspect or delete ${STATE_FILE} to proceed.`)
     }
 }

.serena/memories/l2ps_architecture.md-167-175 (1)

167-175: ⚠️ Potential issue | 🟠 Major

Security model claim may not hold: getL2PSTransactions is accessible to any peer.

Line 171 states "Validator Privacy Leak: IMPOSSIBLE — Validators never receive transaction content." However, the getL2PSTransactions NodeCall endpoint in manageNodeCall.ts (lines 765–808) returns encrypted transaction blobs to any calling peer without verifying L2PS participation. While the content is encrypted, validators can still harvest the ciphertext. Consider reconciling the documentation with the actual access controls or tightening the endpoint.

src/libs/l2ps/L2PSBatchAggregator.ts-535-549 (1)

535-549: ⚠️ Potential issue | 🟠 Major

ZK proof uses synthetic balances — the proof doesn't validate real state.

senderBefore is set to amount and senderAfter to 0, making the constraint senderAfter = senderBefore - amount trivially true for any amount. The proof never checks actual account balances, so it only proves arithmetic consistency of a fabricated scenario.

If this is intentional as a stepping-stone, consider adding a // TODO or doc comment explaining that real balance lookups are needed before this proof carries security weight.

src/libs/network/manageNodeCall.ts-765-808 (1)

765-808: ⚠️ Potential issue | 🟠 Major

getL2PSTransactions exposes encrypted payloads to any peer without access control.

The comment on line 786 says "validators never see this", but this is a public NodeCall endpoint callable by any connected peer. There is no check that the requesting peer is an L2PS participant for the given l2psUid. This could let validators (or any peer) harvest encrypted transaction blobs, undermining the privacy separation described in the architecture docs.

Consider adding a participation check (e.g., verifying the caller's identity against known L2PS participants for that UID) before returning transaction data, or at minimum document why open access is acceptable here.

src/libs/l2ps/L2PSBatchAggregator.ts-523-533 (1)

523-533: ⚠️ Potential issue | 🟠 Major

BigInt(Math.floor(Number(rawAmount))) loses precision for large values.

Converting through Number first truncates integer precision beyond 2^53. Since this represents transaction amounts (potentially large integers), convert directly:

Proposed fix

-                try {
-                    amount = rawAmount !== undefined && rawAmount !== null
-                        ? BigInt(Math.floor(Number(rawAmount)))
-                        : 0n
-                } catch {
-                    amount = 0n
-                }
+                try {
+                    amount = rawAmount !== undefined && rawAmount !== null
+                        ? BigInt(String(rawAmount).split('.')[0])
+                        : 0n
+                } catch {
+                    amount = 0n
+                }

src/libs/l2ps/L2PSBatchAggregator.ts-473-486 (1)

473-486: ⚠️ Potential issue | 🟠 Major

HMAC key derived by slicing raw private key — use a proper KDF instead.

Lines 479–481 extract the first 64 hex characters of the raw private key as the HMAC key. This directly exposes partial private key material as the HMAC secret, and the key isn't domain-separated from the signing key.

Derive a dedicated HMAC key using HKDF (available in Node's crypto module) with a domain-specific info string, e.g.:

Proposed fix

-        const hmacKey = Buffer.from(sharedState.keypair.privateKey as Uint8Array)
-            .toString("hex")
-            .slice(0, 64)
+        const hmacKey = crypto.createHmac("sha256", "L2PS_BATCH_HMAC_KEY_DERIVE")
+            .update(Buffer.from(sharedState.keypair.privateKey as Uint8Array))
+            .digest()

Or better, use crypto.hkdf with a proper salt and info parameter.

scripts/setup-zk-all.ts-37-44 (1)

37-44: ⚠️ Potential issue | 🟠 Major

Missing SHA-256 checksums for L2PS Powers of Tau files.

The identity ptau source has a sha256 checksum for supply-chain integrity verification, but l2ps_16 and l2ps_17 do not. Since verifyPtauChecksum silently skips verification when no checksum is provided (Line 83), these files are downloaded without any integrity guarantee.

src/features/zk/proof/BunSnarkjsWrapper.ts-27-32 (1)

27-32: 🛠️ Refactor suggestion | 🟠 Major

Duplicate ZKProof interface — also defined in ProofVerifier.ts (Lines 28-33).

Both files define identical ZKProof interfaces. Import from one canonical location to avoid divergence.

♻️ Proposed fix

-export interface ZKProof {
-    pi_a: string[]
-    pi_b: string[][]
-    pi_c: string[]
-    protocol: string
-}
+import type { ZKProof } from "./ProofVerifier"

And keep the canonical definition in ProofVerifier.ts.

src/features/zk/scripts/setup-zk.ts-18-19 (1)

18-19: ⚠️ Potential issue | 🟠 Major

Hardcoded /usr/bin/npx breaks portability.

This path won't exist on macOS (/usr/local/bin/npx or Homebrew paths), Windows, or nix-based setups. The unified script scripts/setup-zk-all.ts correctly resolves the local node_modules/.bin/snarkjs binary instead.

🔧 Proposed fix

-// npx path - hardcoded to /usr/bin/npx for reliability
-const NPX = "/usr/bin/npx"
+import { resolve } from "path"
+// Use local snarkjs binary from node_modules for cross-platform compatibility
+const SNARKJS = resolve(__dirname, "../../../node_modules/.bin/snarkjs")

Then replace ${NPX} snarkjs usages with ${SNARKJS}.

src/features/zk/merkle/updateMerkleTreeAfterBlock.ts-159-171 (1)

159-171: ⚠️ Potential issue | 🟠 Major

Rollback fails if no MerkleTreeState exists for the exact target block.

findOne at Line 160 looks for a record where blockNumber === targetBlockNumber. If the target block had no ZK commitments (and thus no tree state was saved for that block), the function throws on Line 168. During a chain reorg, the rollback target may be a block without commitments.

Consider finding the latest state at or before targetBlockNumber instead:

🔧 Proposed fix

             const targetState = await merkleStateRepo.findOne({
                 where: {
                     treeId: GLOBAL_TREE_ID,
-                    blockNumber: targetBlockNumber,
                 },
+                order: { blockNumber: "DESC" },
+                where: {
+                    treeId: GLOBAL_TREE_ID,
+                },
             })
+
+            // Accept any state at or before the target block
+            if (!targetState || targetState.blockNumber > targetBlockNumber) {

src/features/zk/scripts/setup-zk.ts-240-260 (1)

240-260: ⚠️ Potential issue | 🟠 Major

Generate verification keys for both circuits when both compile successfully.

The if-else-if pattern (lines 253–257) causes only identity_with_merkle keys to be generated when both circuits compile. However, test_identity_verification.ts depends on verification_key.json from the basic identity circuit, so skipping its key generation breaks that test. Both circuits should generate keys independently.

Generate keys for both circuits

-        if (merkleCompiled) {
-            await generateKeys("identity_with_merkle")
-        } else if (basicCompiled) {
-            await generateKeys("identity")
-        } else {
+        if (basicCompiled) {
+            await generateKeys("identity")
+        }
+        if (merkleCompiled) {
+            await generateKeys("identity_with_merkle")
+        }
+        if (!basicCompiled && !merkleCompiled) {

src/libs/network/endpointHandlers.ts-805-889 (1)

805-889: ⚠️ Potential issue | 🟠 Major

handleL2PSHashUpdate — solid validation, but the sender is never authenticated.

The method validates the payload structure, block number, and L2PS network participation. However, it does not verify the transaction's cryptographic signature or that tx.content.from is actually a member of the L2PS network. The parallelNetworks.getL2PS(l2psUid) call on Line 845 only checks if the local node has joined the L2PS network — it does not verify the sender's identity or membership. This means any node that knows an L2PS UID the local node participates in could submit arbitrary hash updates.

Consider adding sender signature verification (similar to what handleExecuteTransaction does for standard transactions) before storing the hash.

src/features/zk/types/index.ts-51-65 (1)

51-65: ⚠️ Potential issue | 🟠 Major

siblings type mismatch with MerkleTreeManager.generateProof.

MerkleProofResponse.proof.siblings is typed as string[] (Line 55), but MerkleTreeManager.generateProof() (in src/features/zk/merkle/MerkleTreeManager.ts, Line 156) returns siblings: string[][] (a nested array — each level's sibling is itself an array). This will cause a type error when mapping the proof into this response shape, or silently produce incorrect data if cast.

Align the types — likely siblings should be string[][] here.

Proposed fix

     proof: {
         /** Sibling hashes along the path from leaf to root */
-        siblings: string[]
+        siblings: string[][]
         /** Path indices (0 = left, 1 = right) */
         path_indices: number[]

src/libs/blockchain/transaction.ts-54-79 (1)

54-79: ⚠️ Potential issue | 🟠 Major

Object.assign with ...data does shallow merge — partial content replaces defaults entirely.

If a caller provides new Transaction({ content: { type: "native" } as any }), ...data will replace the entire default content object. Fields like from, to, amount, gcr_edits, and transaction_fee will be missing instead of defaulted. Consider using a deep merge for content, or documenting that callers must provide a complete content object if they override it.

Proposed fix using explicit deep merge for content

     constructor(data?: Partial<ITransaction>) {
+        const defaultContent = {
+            from_ed25519_address: null,
+            type: null,
+            from: "",
+            to: "",
+            amount: null,
+            data: [null, null],
+            gcr_edits: [],
+            nonce: null,
+            timestamp: null,
+            transaction_fee: {
+                network_fee: null,
+                rpc_fee: null,
+                additional_fee: null,
+            },
+        }
         Object.assign(this, {
-            content: {
-                from_ed25519_address: null,
-                type: null,
-                from: "",
-                to: "",
-                amount: null,
-                data: [null, null],
-                gcr_edits: [],
-                nonce: null,
-                timestamp: null,
-                transaction_fee: {
-                    network_fee: null,
-                    rpc_fee: null,
-                    additional_fee: null,
-                },
-            },
+            content: data?.content
+                ? { ...defaultContent, ...data.content,
+                    transaction_fee: { ...defaultContent.transaction_fee, ...data.content.transaction_fee } }
+                : defaultContent,
             signature: null,
             ed25519_signature: null,
             hash: null,
             status: null,
             blockNumber: null,
-            ...data,
+            ...(data ? { ...data, content: undefined } : {}),
         })
     }

Note: This is one approach. Alternatively, you could require callers to always provide a full content and not deep-merge at all, but that should be clearly documented.

src/libs/l2ps/L2PSProofManager.ts-79-94 (1)

79-94: ⚠️ Potential issue | 🟠 Major

Initialization failure is permanent — rejected promise is never cleared.

If init() fails (e.g., database temporarily unavailable), this.initPromise retains the rejected promise and this.repo stays null. Every subsequent call to getRepo() will re-await the same rejected promise and throw. The service cannot recover without a process restart.

🐛 Proposed fix: clear initPromise on failure

     private static async init(): Promise<void> {
         if (this.repo) return
         if (this.initPromise !== null) {
             await this.initPromise
             return
         }

         this.initPromise = (async () => {
-            const dsInstance = await Datasource.getInstance()
-            const ds = dsInstance.getDataSource()
-            this.repo = ds.getRepository(L2PSProof)
-            log.info("[L2PS ProofManager] Repository initialized")
+            try {
+                const dsInstance = await Datasource.getInstance()
+                const ds = dsInstance.getDataSource()
+                this.repo = ds.getRepository(L2PSProof)
+                log.info("[L2PS ProofManager] Repository initialized")
+            } catch (error) {
+                this.initPromise = null
+                throw error
+            }
         })()

         await this.initPromise
     }

src/libs/network/server_rpc.ts-89-122 (1)

89-122: ⚠️ Potential issue | 🟠 Major

Timer leak and dangling async work on timeout in getMerkleTreeManager.

When Promise.race resolves via the timeout branch, two issues arise:

1. The setTimeout timer is never cleared on the success path, keeping it alive until it fires (minor).
2. The original async IIFE continues running after the timeout rejects. If it eventually succeeds, it silently sets globalMerkleManager; if it fails, the rejection is unhandled (no .catch on the dangling promise).

🐛 Proposed fix: track the timer and the inner promise

-    initializationPromise = Promise.race([
-        (async () => {
+    let timeoutId: ReturnType<typeof setTimeout>
+    const initPromise = (async () => {
             const db = await Datasource.getInstance()
             const dataSource = db.getDataSource()
-            // REVIEW: Create local instance, only assign to global after successful init
             const manager = new MerkleTreeManager(
                 dataSource,
                 ZK_MERKLE_TREE_DEPTH,
                 ZK_MERKLE_TREE_ID,
             )
             await manager.initialize()
             log.info("✅ Global MerkleTreeManager initialized")
             globalMerkleManager = manager
             return globalMerkleManager
-        })(),
-        new Promise<MerkleTreeManager>((_, reject) =>
-            setTimeout(() => reject(new Error("Initialization timeout")), INIT_TIMEOUT_MS),
-        ),
-    ])
+    })()
+
+    initializationPromise = Promise.race([
+        initPromise,
+        new Promise<MerkleTreeManager>((_, reject) => {
+            timeoutId = setTimeout(() => reject(new Error("Initialization timeout")), INIT_TIMEOUT_MS)
+        }),
+    ]).finally(() => clearTimeout(timeoutId))
+
+    // Prevent unhandled rejection from the dangling init promise on timeout
+    initPromise.catch(() => {})

src/libs/l2ps/L2PSConsensus.ts-417-465 (1)

417-465: 🛠️ Refactor suggestion | 🟠 Major

Inefficient rollback: fetches all applied proofs then filters in JS; repeated dynamic imports inside loop.

Two issues in rollbackProofsForBlock:

1. Line 420-424: Fetches up to 1000 applied proofs across all blocks, then filters by blockNumber in JS. Push the applied_block_number filter into the database query for efficiency.
2. Lines 457-459: Datasource.getInstance() and the dynamic import are called inside the loop for each proof. Hoist them before the loop.

♻️ Proposed fix

     static async rollbackProofsForBlock(blockNumber: number): Promise<void> {
         try {
-            // Get proofs that were applied in this block
-            const appliedProofs = await L2PSProofManager.getProofs(
-                "", // all L2PS networks
-                "applied",
-                1000
-            )
-
-            // Filter by block number and rollback in reverse order
-            const proofsToRollback = appliedProofs
-                .filter(p => p.applied_block_number === blockNumber)
-                .reverse()
+            // Query proofs applied in this specific block directly
+            const repo = await (await import("@/model/datasource")).default.getInstance()
+            const ds = repo.getDataSource()
+            const proofRepo = ds.getRepository((await import("@/model/entities/L2PSProofs")).L2PSProof)
+
+            const proofsToRollback = (await proofRepo.find({
+                where: { status: "applied" as any, applied_block_number: blockNumber },
+                order: { created_at: "DESC" },
+            }))

             log.info(`[L2PS Consensus] Rolling back ${proofsToRollback.length} proofs for block ${blockNumber}`)

             for (const proof of proofsToRollback) {
                 // ... rollback edits ...

-                const repo = await (await import("@/model/datasource")).default.getInstance()
-                const ds = repo.getDataSource()
-                const proofRepo = ds.getRepository((await import("@/model/entities/L2PSProofs")).L2PSProof)
-
                 await proofRepo.update(proof.id, {
                     status: "pending",
                     applied_block_number: null,
                     processed_at: null
                 })
             }

src/libs/l2ps/L2PSHashService.ts-139-166 (1)

139-166: ⚠️ Potential issue | 🟠 Major

ConnectionPool not shut down during stop().

The OmniProtocol ConnectionPool is initialized in start() but never closed in stop(). This leaks TCP connections and associated resources after shutdown.

🐛 Proposed fix: shut down the connection pool on stop

         if (this.isGenerating) {
             log.warning("[L2PS Hash Service] Forced shutdown - operation still in progress")
         }

+        // Shut down OmniProtocol connection pool if initialized
+        if (this.connectionPool) {
+            try {
+                await this.connectionPool.shutdown()
+            } catch (err) {
+                log.warning("[L2PS Hash Service] Error shutting down connection pool: " + err)
+            }
+            this.connectionPool = null
+        }
+
         log.info("[L2PS Hash Service] Stopped successfully")
         this.logStatistics()

src/features/zk/proof/ProofVerifier.ts-251-280 (1)

251-280: ⚠️ Potential issue | 🟠 Major

Non-transactional path leaks nullifier existence before cryptographic verification.

The fallback path (lines 255-268) checks whether the nullifier is already used before verifying the proof cryptographically. An attacker can submit an invalid proof to probe whether a specific nullifier has been consumed: a "Nullifier already used" response vs. "Proof failed cryptographic verification" reveals the nullifier's state.

Swap the order to match the transactional path — verify cryptographically first, then check the nullifier.

🔒 Proposed fix: reorder non-transactional path

         // Non-transactional fallback (for backward compatibility)
-            // Step 1: Check if nullifier already used
-            const existing = await this.nullifierRepo.findOne({
-                where: { nullifierHash: nullifier },
-            })
-
-            if (existing) {
-                return {
-                    valid: false,
-                    reason: "Nullifier already used (double-attestation attempt)",
-                    nullifier,
-                    merkleRoot,
-                    context,
-                }
-            }
-
-            // Step 2: Cryptographic verification
+            // Step 1: Cryptographic verification (before any DB state checks)
             const cryptoValid = await ProofVerifier.verifyCryptographically(proof, publicSignals)
             if (!cryptoValid) {
                 return {
                     valid: false,
                     reason: "Proof failed cryptographic verification",
                     nullifier,
                     merkleRoot,
                     context,
                 }
             }

-            // Step 3: Validate Merkle root is current
+            // Step 2: Check if nullifier already used
+            const existing = await this.nullifierRepo.findOne({
+                where: { nullifierHash: nullifier },
+            })
+
+            if (existing) {
+                return {
+                    valid: false,
+                    reason: "Nullifier already used (double-attestation attempt)",
+                    nullifier,
+                    merkleRoot,
+                    context,
+                }
+            }
+
+            // Step 3: Validate Merkle root is current
             const rootIsCurrent = await this.isMerkleRootCurrent(merkleRoot)

src/libs/l2ps/zk/L2PSBatchProver.ts-10-23 (1)

10-23: ⚠️ Potential issue | 🟠 Major

Global prototype mutation is risky.

Patching EventTarget.prototype.dispatchEvent at module-load time is a side effect that affects the entire process, not just this module. Any other code relying on standard dispatchEvent behavior could break silently. This also makes the module unsafe to import in contexts where Bun compatibility isn't needed (e.g., tests, tooling).

Consider isolating this patch behind a lazy init or guarding it more tightly (e.g., only when actually about to call snarkjs in Bun), or moving it to a dedicated Bun-shim entrypoint that is imported once at application startup.

src/libs/l2ps/parallelNetworks.ts-301-315 (1)

301-315: ⚠️ Potential issue | 🟠 Major

Unsigned transactions are decrypted with only a warning.

When encryptedTx.signature is falsy (line 313), the code logs a warning but proceeds to decrypt anyway. This means an attacker could submit unsigned (potentially forged) encrypted payloads and have them processed. Consider whether this should be a hard rejection for production.

src/libs/l2ps/parallelNetworks.ts-164-166 (1)

164-166: ⚠️ Potential issue | 🟠 Major

Config UID not validated against the directory UID.

nodeConfig.uid is checked for truthiness but never compared to the uid parameter. A config file in directory foo/ could declare uid: "bar", causing the L2PS instance to be stored under key "foo" in the maps while its internal config says "bar". This mismatch can lead to confusing routing and lookup failures downstream.

Proposed fix

-        if (!nodeConfig.uid || !nodeConfig.enabled) {
-            throw new Error(`L2PS config invalid or disabled: ${uid}`)
+        if (!nodeConfig.uid || nodeConfig.uid !== uid || !nodeConfig.enabled) {
+            throw new Error(`L2PS config invalid, UID mismatch, or disabled: ${uid}`)
         }

src/libs/blockchain/l2ps_mempool.ts-273-278 (1)

273-278: ⚠️ Potential issue | 🟠 Major

Date.now() fallback corrupts the sequence space.

If the non-transactional getNextSequenceNumber path fails, the fallback assigns Date.now() (~1.7 trillion) as a sequence number. All subsequent sequence allocations for this UID will continue from that massive value, creating an irreversible gap. Since the transactional path (inside saveTransactionWithRetry) correctly re-throws on error and retries, this fallback should ideally also throw rather than silently corrupt state.

Proposed fix

             // If in a transaction, we want to rethrow to trigger a retry instead of falling back to timestamp
             if (manager) throw error
 
-            // Fallback to timestamp-based sequence only for non-atomic reads
-            return Date.now()
+            // No safe fallback — propagate error
+            throw error
         }

src/libs/blockchain/l2ps_mempool.ts-395-404 (1)

395-404: ⚠️ Potential issue | 🟠 Major

Error path returns a plausible hash — indistinguishable from valid state.

When getHashForL2PS fails (line 400–404), it returns sha256("L2PS_ERROR_..."), which looks like any other valid hash. Downstream consensus code comparing hashes cannot tell the difference between a genuine state hash and an error artifact, potentially leading to false agreements or disagreements during validation.

Consider throwing the error instead, or returning a distinct sentinel (e.g., empty string) that callers can check:

Proposed fix — let the error propagate

         } catch (error: any) {
             log.error(`[L2PS Mempool] Error generating hash for UID ${l2psUid}, block ${blockNumber}:`, error)
-            // Return deterministic error hash
-            const blockSuffix = blockNumber === undefined ? "_ALL" : `_BLOCK_${blockNumber}`
-            return Hashing.sha256(`L2PS_ERROR_${l2psUid}${blockSuffix}`)
+            throw error
         }

src/libs/blockchain/l2ps_mempool.ts-294-315 (1)

294-315: 🛠️ Refactor suggestion | 🟠 Major

Null-safety: this.repo used without assertion after ensureInitialized.

Throughout this file (lines 310, 328, 378, 427, 461, 507, 553, 588, 609, etc.), this.repo is accessed directly after ensureInitialized(). Since repo is typed as Repository<L2PSMempoolTx> | null, strict TypeScript compilation would flag these as errors. The call on line 310 (this.repo.find(options)) would NPE if repo is somehow still null.

Use the non-null assertion operator (this.repo!) consistently (as done on line 195), or narrow the type via a local guard:

Example fix pattern

+    private static getRepo(): Repository<L2PSMempoolTx> {
+        if (!this.repo) throw new Error("[L2PS Mempool] Not initialized")
+        return this.repo
+    }
+
     // Then use:
-    return await this.repo.find(options)
+    return await this.getRepo().find(options)

src/libs/blockchain/l2ps_mempool.ts-195-198 (1)

195-198: 🛠️ Refactor suggestion | 🟠 Major

(this as any) cast to call own private static method is a code smell.

Line 197 bypasses TypeScript's access control to call getNextSequenceNumber. Since both methods are in the same class and both are private static, this cast shouldn't be necessary. This appears to be a workaround for a TS context issue inside the transaction callback. A cleaner approach is to reference the class by name.

Proposed fix

-                    const sequenceNumber = await (this as any).getNextSequenceNumber(l2psUid, transactionalEntityManager)
+                    const sequenceNumber = await L2PSMempool.getNextSequenceNumber(l2psUid, transactionalEntityManager)